Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: add context to ffi callbacks #6608

Merged
merged 4 commits into from
Oct 8, 2024

Conversation

SWvheerden
Copy link
Collaborator

@SWvheerden SWvheerden commented Oct 8, 2024

Description

Adds c_void context pointer to all FFI callbacks
Splits get address functions into interactive and one-sided

Motivation and Context

By adding a c_void pointer to the all callbacks, each callback can be identified by the context to know from what wallet the callback comes from.
Identifying the callback becomes important if you are running more than one wallet and thus need to identify the calling wallet.

Allows the FFI to get both the interactive and one-sided addresses from the wallet.

Breaking Changes

Adds c_void context to all callbacks.
Renames get_wallet_address to get_wallet_interactive_addess and adds get_wallet_one_sided_address

@SWvheerden SWvheerden requested a review from a team as a code owner October 8, 2024 07:09
Copy link

github-actions bot commented Oct 8, 2024

Test Results (CI)

    3 files    129 suites   37m 7s ⏱️
1 326 tests 1 326 ✅ 0 💤 0 ❌
3 976 runs  3 976 ✅ 0 💤 0 ❌

Results for commit ff494bd.

♻️ This comment has been updated with latest results.

@ghpbot-tari-project ghpbot-tari-project added P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged labels Oct 8, 2024
Copy link

github-actions bot commented Oct 8, 2024

Test Results (Integration tests)

36 tests  +36   36 ✅ +36   14m 57s ⏱️ + 14m 57s
11 suites +11    0 💤 ± 0 
 2 files   + 2    0 ❌ ± 0 

Results for commit ff494bd. ± Comparison against base commit 332068f.

♻️ This comment has been updated with latest results.

brianp
brianp previously approved these changes Oct 8, 2024
Copy link
Contributor

@brianp brianp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the client side what's being used as the context? Any pointer I assume, but specifically, they've decided to use? The wallet pointer?

@ghpbot-tari-project ghpbot-tari-project removed the P-reviews_required Process - Requires a review from a lead maintainer to be merged label Oct 8, 2024
@SWvheerden
Copy link
Collaborator Author

yeah its any pointer, but I dont think you can choose the wallet pointer in the way its constructed. As you need to provide the pointer before you get the wallet?

@ghpbot-tari-project ghpbot-tari-project added the P-reviews_required Process - Requires a review from a lead maintainer to be merged label Oct 8, 2024
@SWvheerden SWvheerden merged commit 3c192c5 into tari-project:development Oct 8, 2024
17 checks passed
@SWvheerden SWvheerden deleted the sw_ffi_context branch October 8, 2024 09:24
sdbondi added a commit to sdbondi/tari that referenced this pull request Oct 10, 2024
* development:
  chore(ci): remove ledger nanos and update ledger key gif resources (tari-project#6617)
  feat: improve mempool error msg when mempool out of sync (tari-project#6618)
  feat: exit logic for pre-mine spend (tari-project#6615)
  chore: new release esme v1.6.0-pre.0 (tari-project#6614)
  feat: enable identity grpc method by default (tari-project#6613)
  feat: pre-mine introduce temp ban and add counters (tari-project#6612)
  chore: changes mainnet default network (tari-project#6610)
  chore(ci): add riscv64 builds and misc script fixes (tari-project#6611)
  feat!: esme test pre-mine with immediate spend (tari-project#6609)
  feat!: add context to ffi callbacks (tari-project#6608)
  feat: add default exclude dial (tari-project#6607)
  feat!: add input mr into genesis block (tari-project#6601)
  feat: update pre_mine specification (tari-project#6606)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants